Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add PSEdition Desktop requirement #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

God-damnit-all
Copy link

I ran Add-Structures.ps1 on PowerShell 7 without knowing any better, which doesn't offer the Get-WmiObject or Checkpoint-Computer commands.

#Requires -PSEdition Desktop will prevent PowerShell 7 from running these scripts since it will require the Windows-specific version of PowerShell: 5.1

@God-damnit-all
Copy link
Author

@damienvanrobaeys Should this be closed too?

@Joly0
Copy link
Contributor

Joly0 commented May 15, 2023

As a note from my side: I think it would be smarter not to block powershell 7 but rather adjust the scripts to use both. For get-wmitobject the get-ciminstance cmdlet could be used instead, they are quite interchangeable.
Regarding the checkpoint-computer cmdlet maybe check if the script is run in powershell >=7 or >=5

if($IsCoreCLR) {
    # im running in pwsh
}
else {
    # running in windows powershell
}

should do the trick for cases where there is no equivalent cmdlet for newer versions of powershell and then give a notification to the user

@God-damnit-all
Copy link
Author

I mean, sure, but this would be a good stopgap until that's sorted out.

@God-damnit-all
Copy link
Author

@damienvanrobaeys How about running Checkpoint-Computer by directly calling powershell.exe and then checking the error code? That would give it compatibility with pwsh.

@Joly0
Copy link
Contributor

Joly0 commented Jun 9, 2023

Imo this is quite a smart idea, could you implement that?

Unfortunately it wont change the fact, that get-wmiobject is not available in pwsh, so we would still need a workaround for that, but i could try to implement that and make a separate pr for it

@damienvanrobaeys
Copy link
Owner

I'm working on it to run checkpoint-computer using powershell if currently running in pwsh

@damienvanrobaeys
Copy link
Owner

Solved this with this:
$Checkpoint_Command = 'Checkpoint-Computer -Description "Windows_Sandbox_Context_menus" -RestorePointType "MODIFY_SETTINGS" -ErrorAction Stop'
$retValue = Start-Process powershell -WindowStyle Hidden -ArgumentList $Checkpoint_Command -wait -PassThru
If($retValue.ExitCode -eq 0)
{
Write_Log -Message_Type "SUCCESS" -Message "Creation of restore point "Add Windows Sandbox Context menus""
}
Else
{
Write_Log -Message_Type "ERROR" -Message "Creation of restore point "Add Windows Sandbox Context menus""
}

@Joly0
Copy link
Contributor

Joly0 commented Jun 9, 2023

Why the hassle with if/else? Just use try/catch and you dont need a separate variable for the whole command

@God-damnit-all
Copy link
Author

Imo this is quite a smart idea, could you implement that?

Unfortunately it wont change the fact, that get-wmiobject is not available in pwsh, so we would still need a workaround for that, but i could try to implement that and make a separate pr for it

There shouldn't be anything that Get-WmiObject can do that Get-CIMInstance can't, to my knowledge, and Get-CIMInstance is compatible with both.

@Joly0
Copy link
Contributor

Joly0 commented Jun 9, 2023

Ye, that is true. So i guess try/catch for checkpoint-computer and get-ciminstance should make this compatible with pwsh and remain it with powershell

@damienvanrobaeys
Copy link
Owner

I updated add_structure.ps1 with checkpoint-computer running with start-process powershell for al versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants